Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Cleanup cmake rules #566

Merged
merged 2 commits into from
May 5, 2018
Merged

Conversation

StefanBruens
Copy link
Contributor

Currently, the whole documentation gets build twice when doing "make && make install", as all files are created using commands specified in "add_custom_target".

As documented for add_custom_target, these commands are always executed. The correct way to only rebuild on changes is a combination of add_custom_target(target DEPENDS file) and add_custom_command(OUTPUT file COMMANDS ...).

@StefanBruens
Copy link
Contributor Author

Fixes #73
Don't know why the build fails here, a local build works fine

@nickoe
Copy link
Contributor

nickoe commented May 4, 2018

Did you try a clean build?

@ciampix
Copy link
Collaborator

ciampix commented May 4, 2018

I just cloned your repo and confirmed that it works as expected... don't know what is going on here ... I'm for applying it ... @nickoe ?

@ciampix
Copy link
Collaborator

ciampix commented May 4, 2018

to @nickoe: are we sure that Travis do correctly clean up before building?

@nickoe
Copy link
Contributor

nickoe commented May 4, 2018

Yes, I am pretty sure. that it the intention. Are you building the same way as travis?

mkdir build && cd build && cmake .. && make -j4

@ciampix
Copy link
Collaborator

ciampix commented May 4, 2018

Yes definitely

@ciampix
Copy link
Collaborator

ciampix commented May 4, 2018

May I go ahead and merge?

@StefanBruens
Copy link
Contributor Author

I also use the same command sequence to build

@StefanBruens
Copy link
Contributor Author

Just had an idea whats going wrong here - I only build html and pdf, but not epub.
Both epub and pdf use an intermediate adoc -> docbook step, and executing both in parallel likely corrupts the docbook xml file.

a) add a dependency to force serialization
b) add an additional rule to create the docbook file, and feed the docbook file into a2x (explicitly supported by a2x)

@ciampix
Copy link
Collaborator

ciampix commented May 4, 2018

Ok try commit and lets see what it happens ... BTW the above command build all docs in parallel and I didn't see any problem at all...

@StefanBruens
Copy link
Contributor Author

Can you just wait a little bit - I have fixed it locally and just need to cleanup

…ilds

As stated in the add_custom_target documentation, the target "is always
considered out of date". Instead, only add the output file as a target
dependency and create the file using add_custom_command. With this change
the documentation is only recreated when the input changes.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Instead of creating custom target for each intermediate output, like
translated asciidocs, just add/append the generated output files as a
dependency.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
@StefanBruens StefanBruens force-pushed the cleanup_cmake_rules branch from 69beacd to 1a7bc75 Compare May 5, 2018 03:46
@ciampix ciampix merged commit f67683b into KiCad:master May 5, 2018
@ciampix
Copy link
Collaborator

ciampix commented May 5, 2018

Great! Thanks @StefanBruens!

@nickoe
Copy link
Contributor

nickoe commented May 13, 2018

Now the build time went from 2 hr 9 min to about 43 min. Thanks.

@nickoe
Copy link
Contributor

nickoe commented May 22, 2018

@StefanBruens Will you be able to throw some tips on KiCad/kicad-i18n#208 ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants